Skip to content

remove append #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 5, 2022
Merged

remove append #261

merged 10 commits into from
Sep 5, 2022

Conversation

twoertwein
Copy link
Member

As long as a class as __getattr__ exist, mypy and pyright assume that .append (and any other non-existing method) might still exists. Removing __getattr__ has the disadvantage that users can no longer do df.column_name.

@twoertwein twoertwein requested a review from Dr-Irv September 4, 2022 16:19
def test_getattr() -> None:
# GH 261
series = pd.Series([1, 2, 3], index=["a", "b", "c"], dtype=int)
check(assert_type(series.a, int), np.int64)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately, it doesn't return a builtin int

@twoertwein
Copy link
Member Author

The error on windows could be a pandas/numpy inconsistency. It seems that our windows CI is 64bit (np.intp is np.int64) but the observed dtype is np.int32.

@twoertwein twoertwein requested a review from Dr-Irv September 4, 2022 23:57
@@ -359,7 +360,7 @@ class NDFrame(PandasObject, indexing.IndexingMixin):
self, func: Callable[..., T] | tuple[Callable[..., T], str], *args, **kwargs
) -> T: ...
def __finalize__(self, other, method=..., **kwargs) -> NDFrame: ...
def __getattr__(self, name: _str): ...
def __getattr__(self, name: _str) -> Series: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to core/frame.pyi and you can avoid the # type: ignore in core/series.pyi

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @twoertwein

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants